fix(#392): 브랜드 상세 조회 API에서 브랜드의 유저의 ID가 나올 수 있도록 변경#393
Conversation
Summary of ChangesHello @Yoonchulchung, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 브랜드 상세 조회 API의 응답에 브랜드 소유자의 사용자 ID를 포함하도록 개선합니다. 기존에는 브랜드 정보만 제공되었으나, 이제는 해당 브랜드를 등록한 사용자의 ID까지 함께 제공되어 API 사용자가 브랜드와 사용자 간의 관계를 명확하게 파악할 수 있게 됩니다. 이는 특정 브랜드와 관련된 사용자 정보를 필요로 하는 기능에 유용할 것입니다. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR introduces changes to include the brand owner's user ID in the brand detail API response. However, a high-severity information disclosure vulnerability has been identified. Exposing internal user IDs can lead to Insecure Direct Object Reference (IDOR) attacks and user enumeration. It is strongly recommended to remediate this by not exposing internal identifiers in API responses, especially in DTOs where fields should only be added if securely required by a consumer. If a public identifier is required, consider using a non-sequential, non-guessable alternative. Additionally, if an ID were to be exposed, ensuring consistency in data types, such as using Long for ID fields, would be beneficial.
| BrandDetailResponseDto.BrandDetailResponseDtoBuilder responseBuilder = BrandDetailResponseDto.builder() | ||
| .userId(currentUserId) | ||
| .brandName(brand.getBrandName()) | ||
| .brandUserId(String.valueOf(brand.getUser().getId())) |
There was a problem hiding this comment.
This line exposes the internal userId of the brand's owner, leading to a high-severity information disclosure vulnerability. This can enable Insecure Direct Object Reference (IDOR) attacks and user enumeration. It is strongly recommended to avoid exposing internal database identifiers in API responses. If a unique identifier is required, use a non-sequential, non-guessable public identifier (e.g., a UUID). The code review also suggested changing the type from String to Long for brandUserId to align with brand.getUser().getId()'s return type, but the primary concern is the exposure of the ID itself.
| public class BrandDetailResponseDto { | ||
| private Long userId; | ||
| private String brandName; | ||
| private String brandUserId; |
There was a problem hiding this comment.
This brandUserId field is intended to carry a sensitive internal user ID. Exposing internal user IDs in API responses is a high-severity security risk, potentially leading to Insecure Direct Object Reference (IDOR) vulnerabilities and other attacks. It is strongly recommended to remove this field entirely to avoid exposing internal identifiers. Additionally, as per repository rule 'Avoid adding fields to a DTO if they are not currently required by a consumer', this field should not be present if it's not securely required by a consumer. While the code review suggested changing its type to Long for consistency, the primary concern is the exposure of the ID itself.
References
- Avoid adding fields to a DTO if they are not currently required by a consumer, even if they were present in a previous version.
1 participant
Summary
브랜드 상세 조회 API에서 브랜드의 유저의 ID가 나올 수 있도록 변경
Changes
Type of Change
Related Issues
#392